refactor(standards): split fee estimation out of pay_fee - #3796
Open
mmagician wants to merge 12 commits into
Open
refactor(standards): split fee estimation out of pay_fee#3796mmagician wants to merge 12 commits into
pay_fee#3796mmagician wants to merge 12 commits into
Conversation
This was referenced Sep 3, 2026
mmagician
commented
Sep 3, 2026
mmagician
force-pushed
the
mmagician-claude/port-fee-estimate-split
branch
from
September 3, 2026 10:45
ccad489 to
05ccca0
Compare
mmagician
commented
Sep 3, 2026
mmagician
commented
Sep 3, 2026
mmagician
commented
Sep 3, 2026
mmagician
changed the title
pay_fee
mmagician
marked this pull request as draft
September 3, 2026 12:21
mmagician
marked this pull request as ready for review
September 3, 2026 14:03
Port of #3784 from release/v0.16.0-rc to next. `fee::estimate_fee` owns everything up to the single `tx::compute_fee`, pricing the sponsorship notes through the new `fees::estimate_network_note_sponsorships`; `pay_fee` composes it with `fees::create_network_note_sponsorships` and the unchanged payment tail. Adapted to next: `tx::get_fee_asset_id` replaces `get_fee_faucet_id`, which made the `pay_network_note_sponsorships` wrapper a single call, so it was dropped as suggested in the rc review; `to_amount_unchecked` follows the next rename. Unlike rc, `pay_fee` reads the fee asset ID once for both passes and the margin math reuses the output-note count the walk already read: the rc shape added two kernel calls per fee payment, which pushed the "consume two P2ID notes with ECDSA signing" bench scenario past the 16384-row padding bracket pinned by the trace-contract guard. Cost tables regenerated on next. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
…he apply_cycle_margins change Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
Drops the wrapper that only read the asset ID: `fee::estimate_fee` now takes FEE_ASSET_ID as its first input, and `pay_fee` reads it once for the estimate and the sponsorship payment. Cost tables regenerated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
The procedure gained the sponsorship-note count as well as the output-note count. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
Co-authored-by: Marti <marcin.gorny.94@protonmail.com>
The estimate pass records every network output note's price in a caller-provided table indexed by output note index, and the creation pass funds the sponsorship notes from that table instead of repeating the FPI call into each target's fee policy. `pay_fee` keeps the table in its locals; `estimate_fee` and both `fees` walks take its address. This removes the second pricing pass from the payment tail, so the cycle margins after `compute_fee` no longer stand in for foreign procedure calls, and network-note transactions shrink by about 3500 core rows. Cost tables regenerated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
Covers the price table with two network notes priced differently, so a sponsorship funded from the wrong entry would fail. Also names the table size on estimate_fee's doc and the changelog, and drops a stale comment about a pricing call in the payment tail. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
The price table holds the fee asset value word each target returned, one per output note, so the payment reads back exactly what was priced instead of rebuilding the value from an amount. Cost tables regenerated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
With the pricing call gone from the payment tail, a sponsorship note costs about 4300 cycles to create and each walked output note about 400, measured by diffing the auth procedure cycles of transactions with one to three plain, zero-priced and sponsored network notes. SPONSORSHIP_NOTE_CYCLES drops from 16384 to 8192; the walk margin stays. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
mmagician
force-pushed
the
mmagician-claude/port-fee-estimate-split
branch
from
September 3, 2026 14:12
470a84a to
92e00db
Compare
Asserts the measured creation cost of one sponsorship note stays within SPONSORSHIP_NOTE_CYCLES and the cost of walking one more output note through both passes within twice the per-note walk margin, mirroring the constants next to the other fee-flow mirrors. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KkQzFtRjyDwQ7iVtRbsHsn
bobbinth
approved these changes
Sep 3, 2026
bobbinth
left a comment
bobbinth
left a comment
Contributor
There was a problem hiding this comment.
Looks good! Thank you! I left a couple of comment-related comments inline.
Comment on lines
+710
to
+714
| #! With create_notes = 0 the note is priced through its target account's fee policy and the fee | ||
| #! asset value is recorded in the note's price table entry. With create_notes = 1 the recorded value | ||
| #! is read back and, if it is non-zero, the sponsorship note is created and funded with it in the | ||
| #! expected fee asset. A network note the target account prices to zero needs no sponsorship note, | ||
| #! so none is created for it. |
Contributor
There was a problem hiding this comment.
It may be worth the explicitly state that if we call this procedure with create_notes = 1, we expect the price table to have been populated via a previous call to this procedure with create_notes = 0.
Comment on lines
+457
to
+461
| exec.estimate_fee | ||
| # => [fee_amount, FEE_ASSET_ID, price_table_ptr, CONVERSION_INFO] | ||
|
|
||
| # no output notes are excluded from the fee computation | ||
| padw movup.4 | ||
| # => [num_estimated_extra_cycles, EXCLUDE_NOTES_COMMITMENT, CONVERSION_INFO, | ||
| # total_sponsored_fee_amount] | ||
| movdn.5 exec.fees::create_network_note_sponsorships | ||
| # => [total_sponsored_fee_amount, fee_amount, CONVERSION_INFO] |
Contributor
There was a problem hiding this comment.
Related to the above comment: a call to fees::create_network_note_sponsorships works correctly here because we first call estimate_fee (which calls fees::estimate_network_note_sponsorships) - right? If so, I think this is pretty subtle and would be good to add a comment that the call to estimate_fee populates the price table and then the call to fees::create_network_note_sponsorships uses this price table (we do have a comment about it at the beginning of the procedure, but I'd add something like "fill the price table" above exec.estimate_fee and "create notes using the price table" above exec.fees::create_network_note_sponsorships).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Port of #3784 from
release/v0.16.0-rctonext.Changes w.r.t. #3784
next:tx::get_fee_faucet_idbecametx::get_fee_asset_id(#3741), which returns the asset ID directly, so the rcpay_network_note_sponsorshipswrapper collapsed to a single call and was dropped as suggested in the rc review.fungible_asset::to_amount_uncheckedfollows thenextrename.pay_feecalledestimate_fee, which readtx::get_fee_faucet_idto run the estimate, and then calledpay_network_note_sponsorships, which read it again to run the creation. Onnext, the port now readstx::get_fee_asset_idonce inpay_fee, duplicates the word on the stack, and passes one copy to estimate/create respectively.Notes
The two follow-ups originally listed here (zero-priced network notes budgeted at the 512-cycle walk margin while the payment pass still priced them via FPI after
compute_fee, and the double pricing pass adding about 3950 core rows to network-note transactions) are addressed by b0e0179: the estimate records each network note's price inpay_fee's local memory and the payment reads it back, so every network note is priced once and the payment tail no longer contains a foreign procedure call. "consume CLAIM note (L2 to Miden)" is back to 4378 rows under its 65536 bracket.